-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added inheritance filter to gemini plugin #223
base: master
Are you sure you want to change the base?
Conversation
Finally works!! Please review @brainstorm @guillermo-carrasco @robinandeer |
c3ac6d9
to
aa9011e
Compare
@@ -53,6 +53,12 @@ def gemini_path(request): | |||
gemini_db = "tests/fixtures/HapMapFew.db" | |||
return gemini_db | |||
|
|||
@pytest.fixture(scope='function') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this decorator tell you when that "big" GEMINI db is not there?
Would it make sense to be a bit smarter here and pull the S3 file when and if it's not present while running the testsuite? Could save some time for new devs/beginners before they figure it out when reading the docs...
Automation >> Docs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry I added this then changed my mind but forgot to remove. Thanks for noting!
We should have a dedicated test database as issued in #225
Cool stuff Måns! Just went through the PR, seems fine to me except the HapMapMany test/comment I left... It would make sense to have an exception for pulling the data from S3: When tests are running on TravisCI, do not pull those in. Other than that, excellent job! ;) |
I have no idea why the tests fails, it's working on my local installation... |
@robinandeer would you like to have a last review of this? |
- Add a .coveragec file to control what code is excluded when checking test coverage. Modify this file if there is repeated types of code that should be excluded from coverage testing. - Remove coverage testing when using gemini.gim since it does not work.
hmm I don't even get what's the problem exactly, the stacktrace is confusing :-/ |
@guillermo-carrasco think that the problem has to do with gemini.gim uses |
How are we standing on this? Doesn't GEMINI do a test for this case or are they not using py.test? I don't like the idea of merging in code that will make tests fail just bc having a "broken master branch" tag in the README is not a good thing for potential collaborators etc. to see @moonso @brainstorm @guillermo-carrasco @henrikstranneheim ?? |
I had to deal with GEMINI's "testsuite" in PR arq5x/gemini#719... TL;DR: Not fun. Best chance you guys get is pinging BrentP on this if you are really stuck, IMHO... |
Yeah let's bring in BrentP and see what he has to say Otherwise I guess we'll drop that unit test and/or work around it 😕 |
No description provided.